Skip to content

improvement(knowledge): walk connector reconciliation by id so seen stamps stay off the index - #8334

Merged
waleedlatif1 merged 1 commit into
stagingfrom
improvement/connector-reconciliation-id-keyset
Sep 26, 2026
Merged

waleedlatif1 merged 1 commit into
stagingfrom
improvement/connector-reconciliation-id-keyset

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Connector sync stamps document.source_seen_at on every listed document. source_seen_at is a key column of doc_connector_reconciliation_idx, so every stamp was a non-HOT update that wrote every index on document. This is release A of two: move every reader off that key so release B can drop it and the stamps become HOT
  • New doc_connector_reconciliation_v2_idx (connector_id, id) with the same partial predicate, built concurrently. The old index stays until release B
  • The absence walks (ACL revoke, soft delete, hard delete) and the member resurrection walk page by id instead of (seen, id), with absence as a plain filter. Each window is built by a recursive keyset walk that fetches one row per step (id > previous ORDER BY id LIMIT 1) up to 5,000 ids, so every statement reads at most one window whatever plan the database picks. A single ORDER BY id LIMIT could be planned as a bitmap read of the whole connector plus a sort. Matches are filtered within the window, so cost is bounded by ids scanned, not matches found. A walk whose absence count is zero is skipped
  • Both seen stamps skip rows already stamped at or after the run start (staleSeen), so current rows aren't rewritten and a later stamp is never overwritten by an earlier one

Type of Change

  • Improvement

Testing

  • New listing-continuation case: 15k present rows, then a few absent live rows and tombstones last in id order. Every absent row is processed, and every page statement (re-run under EXPLAIN ANALYZE) stays within the window bound. It fails on the unbounded id keyset (15,003 rows scanned in one statement)
  • Seen-stamp guard: rows stamped after the run start keep their stamp (connector-lease-pages); it fails with the guard removed
  • Scale suite: both window statements use the v2 index. Reconciliation took 2.0s vs 15.9s with unbounded pages
  • listing-continuation (incl. NULL and microsecond tie cases), member-document-lifecycle, sync-content-pass, and connector-lease-pages pass, as do the lib/knowledge unit tests, type-check, lint, check:audits, and check:migrations origin/staging
  • Release B, after this deploys: drop doc_connector_reconciliation_idx concurrently and add a HOT-update regression test
  • Migration renumbered to 0388 on top of staging

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Sep 26, 2026 11:39pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 16 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/knowledge/connectors/reconciliation-window.ts
Comment thread packages/db/schema.ts
@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Adds database index and changes reconciliation walk logic.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR adds a concurrent (connector_id, id) reconciliation index and moves absence and resurrection walks to bounded id windows. It also guards seen stamps against rewriting rows already stamped for the run, with integration and scale coverage. The existing seen-keyed index remains for a later release.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Completed connector listing] --> B[Count absence and apply safety holds]
  B --> C[Scan up to 5,000 owned documents by id]
  C --> D[Filter matches within window]
  D --> E[Revoke ACLs or remove documents in pages]
  E --> F{More ids?}
  F -->|Yes| C
  F -->|No| G[Finish reconciliation]
Loading

Reviews (5) · Last reviewed commit: "improvement(knowledge): walk connector r..."

Comment thread apps/sim/lib/knowledge/connectors/reconciliation-window.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 17 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 17 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 force-pushed the improvement/connector-reconciliation-id-keyset branch from 735f70d to cdd6e8c Compare September 26, 2026 21:55
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 18 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 force-pushed the improvement/connector-reconciliation-id-keyset branch from cdd6e8c to 9b2d1d8 Compare September 26, 2026 23:37
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 18 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit eeaa57b into staging Sep 26, 2026
32 of 33 checks passed

This branch was successfully deployed

1 active deployment
Preview — 9b2d1d81 Deployed Sep 26, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant